feat: add targetRef field to ZTunnel CRD#1259
feat: add targetRef field to ZTunnel CRD#1259istio-testing merged 1 commit intoistio-ecosystem:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1259 +/- ##
==========================================
- Coverage 81.26% 81.19% -0.08%
==========================================
Files 46 46
Lines 2493 2542 +49
==========================================
+ Hits 2026 2064 +38
- Misses 344 349 +5
- Partials 123 129 +6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bcd9f0d to
b655b0e
Compare
|
This should support most use cases including RevisionBased update strategies already. Another thing we could add is making the This could be inferred at install time by looking for a |
| return fmt.Errorf("failed to apply profile: %w", err) | ||
| } | ||
|
|
||
| // Apply values from referenced IstioRevision first |
There was a problem hiding this comment.
Maybe it would make sense to warn users when setting the same config in both ztunnel and Istio to different values?
There was a problem hiding this comment.
I don't think we need to warn users - their settings will always take precedence. By default, they should only configure the version and targetRef
There was a problem hiding this comment.
Yeah, a warning might imply that something is wrong. Can we add a log message saying that user setting from ztunnel are used?
There was a problem hiding this comment.
hmm. as we're just merging the fields I don't think we have the ability to give that sort of granular feedback. or rather, we'd have to add a bunch of logic in order to do that. maybe just documenting the behavior is enough?
| BeforeAll(func() { | ||
| common.CreateIstio(k, version.Name, ` | ||
| values: | ||
| global: |
There was a problem hiding this comment.
Should this be added to samples?
There was a problem hiding this comment.
I'm not sure about the global setting. I will add the targetRef to the sample though
| return fmt.Errorf("failed to apply profile: %w", err) | ||
| } | ||
|
|
||
| // Apply values from referenced IstioRevision first |
There was a problem hiding this comment.
Yeah, a warning might imply that something is wrong. Can we add a log message saying that user setting from ztunnel are used?
| var revisionName string | ||
| switch ref.Kind { | ||
| case v1.IstioRevisionKind: | ||
| revisionName = ref.Name |
There was a problem hiding this comment.
Should we add input validation for targetReference?
| version: %s | ||
| namespace: %s | ||
| targetRef: | ||
| kind: Istio |
There was a problem hiding this comment.
Do you think we will require a test case without the targetRef?
There was a problem hiding this comment.
we have tests that verify that user input takes precedence so it should be okay
This is a good idea. |
|
Hi @dgn, do you think it could be useful to add info about this API change into ZTunnel CRD docs too? |
88ed7f4 to
926dd12
Compare
done! |
| spec: | ||
| version: v1.29.0 | ||
| namespace: ztunnel | ||
| targetRef: |
There was a problem hiding this comment.
Can you remind me if we wanted to populate the spec.version as well from the targetRef?
There was a problem hiding this comment.
I considered it, but I guess we'd want to have a placeholder version in that case? (e.g. "fromParent")
8796de3 to
28e28a5
Compare
Done! |
|
All comments addressed, PTAL 🙂 |
sridhargaddam
left a comment
There was a problem hiding this comment.
Mostly good - just some minor comments.
16ede15 to
09b6637
Compare
2f830ed to
c7984c5
Compare
|
I've added "/hold" so that @nrfox (or others) can take a final look. Feel free to remove the hold if needed. |
|
/retest |
This adds a `targetRef` field to the ZTunnel CRD, allowing users to keep their ZTunnel config in sync with what they already defined in the `Istio` or `IstioRevision` CRD. Signed-off-by: Daniel Grimm <dgrimm@redhat.com>
* upstream/main: Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1848) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1845) feat: add targetRef field to ZTunnel CRD (istio-ecosystem#1259) Add 1.29.2 and 1.28.6 versions (istio-ecosystem#1844) Use typed Go structs for FIPS values instead of helm.Values (istio-ecosystem#1695) Add helm.sh/helm/v3 to license allowlist alongside v4 (istio-ecosystem#1840) Update getLatestVersionByPrefix helm to v4 on update_deps.sh (istio-ecosystem#1833) Update kustomization files with registry.istio.io (istio-ecosystem#1829) Improve testing images tags for OLM and Operator images (istio-ecosystem#1819) Automator: Update EOL Istio versions in istio-ecosystem/sail-operator@main (istio-ecosystem#1821) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1820) Migrate from Helm v3 to Helm v4 (istio-ecosystem#1644) introduce crd-schema-checker (istio-ecosystem#1055) Add kubebuilder validation for revisionTagTargetRef (istio-ecosystem#1774) Handle errors in Helm discovery client (istio-ecosystem#1812) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1808)
* upstream/main: Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1851) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1850) Add documentation for resource customization (istio-ecosystem#1292) refactor error and status condition handling (istio-ecosystem#1807) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1848) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1845) feat: add targetRef field to ZTunnel CRD (istio-ecosystem#1259) Add 1.29.2 and 1.28.6 versions (istio-ecosystem#1844) Use typed Go structs for FIPS values instead of helm.Values (istio-ecosystem#1695) Add helm.sh/helm/v3 to license allowlist alongside v4 (istio-ecosystem#1840) Update getLatestVersionByPrefix helm to v4 on update_deps.sh (istio-ecosystem#1833) Update kustomization files with registry.istio.io (istio-ecosystem#1829) Improve testing images tags for OLM and Operator images (istio-ecosystem#1819) Automator: Update EOL Istio versions in istio-ecosystem/sail-operator@main (istio-ecosystem#1821) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1820) Migrate from Helm v3 to Helm v4 (istio-ecosystem#1644) introduce crd-schema-checker (istio-ecosystem#1055) Add kubebuilder validation for revisionTagTargetRef (istio-ecosystem#1774) Handle errors in Helm discovery client (istio-ecosystem#1812) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1808)
* upstream/main: (23 commits) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1856) Modify "download-charts" script for alpha/beta releases (istio-ecosystem#1852) Add operator `TLSConfig` and sync with APIServer TLS profile on openshift (istio-ecosystem#1513) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1851) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1850) Add documentation for resource customization (istio-ecosystem#1292) refactor error and status condition handling (istio-ecosystem#1807) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1848) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1845) feat: add targetRef field to ZTunnel CRD (istio-ecosystem#1259) Add 1.29.2 and 1.28.6 versions (istio-ecosystem#1844) Use typed Go structs for FIPS values instead of helm.Values (istio-ecosystem#1695) Add helm.sh/helm/v3 to license allowlist alongside v4 (istio-ecosystem#1840) Update getLatestVersionByPrefix helm to v4 on update_deps.sh (istio-ecosystem#1833) Update kustomization files with registry.istio.io (istio-ecosystem#1829) Improve testing images tags for OLM and Operator images (istio-ecosystem#1819) Automator: Update EOL Istio versions in istio-ecosystem/sail-operator@main (istio-ecosystem#1821) Automator: Update dependencies in istio-ecosystem/sail-operator@main (istio-ecosystem#1820) Migrate from Helm v3 to Helm v4 (istio-ecosystem#1644) introduce crd-schema-checker (istio-ecosystem#1055) ...
This adds a
targetReffield to the ZTunnel CRD, allowing users to keep their ZTunnel config in sync with what they already defined in theIstioorIstioRevisionCRD.